perf: reuse an already-correctly-capped dataset instead of re-simulating it - #476
Merged
Merged
Conversation
…ing it should_simulate's PYAUTO_SMALL_DATASETS=1 branch deleted and re-simulated every dataset unconditionally. That was correct when written, and the docstring said why: it "cannot know the capped dataset on disk was produced by the SAME cap, so it always regenerates". The SMALLDAT stamp (PyAutoNerves#153) removes exactly that limitation, so the branch was paying for a full simulation pass per dataset per smoke run across ~253 call sites, for datasets that were already correct. Reuse now requires positive evidence on BOTH counts: stamped SMALLDAT = T AND measuring exactly SMALL_DATASETS_SHAPE_NATIVE. The second half is the trap, and dropping it would be worse than not doing this at all. SMALLDAT = T means "capped at whatever the cap was when this was written", NOT "capped at today's cap". If SMALL_DATASETS_SHAPE_NATIVE is ever changed, every dataset on disk goes on claiming T at the old size, and reusing on the stamp alone would silently feed stale wrong-sized data to a run that asked for the new cap -- the same silent-stale-dataset bug the stamp was introduced to prevent, reintroduced through the opposite branch. Pinned by test. This is the mirror image of _stamp_contradicted_by_shape on the full-resolution branch and exists for the same reason: the stamp records the writer's environment, not a measured property of the data. Neither branch may treat it as unfalsifiable. Interferometer datasets deliberately never qualify and keep regenerating every run. Their data.fits is (n_visibilities, 2) -- shape fixed by the committed uv file, unchanged by the cap -- so shape cannot corroborate their stamp, and the alternative is trusting the stamp alone for precisely the family whose corruption is invisible. Stated in the docstring rather than left to fall out of the code. Everything without a readable top-level data.fits (JSON-only, datacubes nesting theirs in channel_XXX/, multi_dataset's prefixed names) also fails the check and regenerates, preserving today's behaviour for the families this cannot speak about. The existing test asserting the unconditional behaviour encoded the limitation being removed; rewritten, plus cases for each trap above.
Jammy2211
added
the
pending-release
Claude
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to PyAutoNerves#153 / #474, which added the
SMALLDATstamp and deliberately left this branch alone.should_simulate'sPYAUTO_SMALL_DATASETS=1branch deleted and re-simulated every dataset unconditionally. That was correct when written, and the docstring said exactly why: it "cannot know the capped dataset on disk was produced by the SAME cap, so it always regenerates." The stamp removes that limitation, so the branch was paying for a full simulation pass per dataset per smoke run — across ~253should_simulatecall sites in autolens_workspace — for datasets that were already correct.Reuse now requires positive evidence on both counts: stamped
SMALLDAT = Tand measuring exactlySMALL_DATASETS_SHAPE_NATIVE.Why the second half is not optional
Dropping it would be worse than not doing this at all, and it is why this wasn't the one-liner it looked like.
SMALLDAT = Tmeans "capped at whatever the cap was when this file was written", not "capped at today's cap". IfSMALL_DATASETS_SHAPE_NATIVEis ever changed, every dataset already on disk goes on claimingTat the old size. Reusing on the stamp alone would then silently feed stale, wrong-sized data to a run that asked for the new cap — the same class of silent-stale-dataset bug the stamp was introduced to prevent, reintroduced through the opposite branch. Pinned by test.This is the mirror image of
_stamp_contradicted_by_shapeon the full-resolution branch, and it exists for the same reason: the stamp records the writer's environment, not a measured property of the data. Neither branch may treat it as unfalsifiable.Behaviour matrix
Tbut at a different capSMALLDAT = F)(n_vis, 2)data.fitsOnly the first row changes. Everything else keeps today's behaviour, because reuse requires positive evidence and everything else fails to provide it.
Interferometer datasets deliberately never qualify. Their
data.fitsis(n_visibilities, 2)— shape fixed by the committed uv file, unchanged by the cap — so shape cannot corroborate their stamp. Rather than trust the stamp alone for precisely the family whose corruption is invisible, they keep regenerating every run. That decision is written into the docstring rather than left to fall out of the code.API Changes
None — internal changes only.should_simulate's signature is unchanged. The only behavioural change is skipping a re-simulation that produced a byte-equivalent dataset.Test Plan
python -m pytest test_autoarray/— 1106 passed, 64 skipped, 0 failedPYAUTO_SMALL_DATASETS=1exported and unset; tree clean both waysTis regenerated — the trap abovedata.fitscases all regenerateFull API Changes (for automation & release notes)
Removed / Renamed / Changed Signature
None.
Added (private)
autoarray.util.dataset_util._is_capped_at_the_current_cap(dataset_path)—Trueonly whendata.fitsis stampedSMALLDAT = Tand its shape is exactlySMALL_DATASETS_SHAPE_NATIVE.Changed Behaviour
should_simulate(dataset_path)— thePYAUTO_SMALL_DATASETS=1branch keeps a dataset already capped to the current cap instead of deleting it. Full-resolution branch untouched.Migration
None.
Generated by the PyAutoLabs agent workflow.
Generated by Claude Code